-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CSHARP-4455: Fix test assertation. #994
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
// in rare cases, the thrown error is "HttpRequest exception: AcceessDeniedException". That means you don't have authorization to perform the requested action. | ||
// It more or less corresponds to the expected behavior here, but it's unclear why the same scenario triggers different exceptions. | ||
// However, it looks harmless to slightly update the test assertation to avoid assertation failures on EG | ||
AssertInnerEncryptionException<HttpRequestException>(ex, "Error in KMS response. HTTP status=400. Response body=\n{\"__type\":\"AccessDeniedException\"}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% confident that thrown exception in this (and similar) patches is HttpRequestException
. But if no, we will see it and will be able to fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will try to reproduce it before merging, but given that I saw this exception around 5 times in total during last few months, my attempts might be not enough to do it for sure, but at least we can try
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
found and fixed. See for details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, minor comments.
...Driver.Tests/Specifications/client-side-encryption/prose-tests/ClientEncryptionProseTests.cs
Outdated
Show resolved
Hide resolved
...Driver.Tests/Specifications/client-side-encryption/prose-tests/ClientEncryptionProseTests.cs
Outdated
Show resolved
Hide resolved
…on/prose-tests/ClientEncryptionProseTests.cs Co-authored-by: BorisDog <[email protected]>
…on/prose-tests/ClientEncryptionProseTests.cs Co-authored-by: BorisDog <[email protected]>
No description provided.